You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds Workflow Functions: project-installed Python callables that workflow expressions can use as pipe filters ({{ steps.x.output.stdout | trim }}). Closes#5285; the direction and scope were agreed with @mnriem in the issue ("Go for it!").
.specify/workflows/functions/<id>/
function.yml # id, functions: [{name, handler, min_args, max_args, description}]
__init__.py # the handlers; called as handler(value, *args)
specify workflow functionadd text --dev ./text # or --from <archive-url> [--force]
specify workflow functionlist [--json] # registry metadata only, never imports code
specify workflow functionremove text
Expression engine.FilterSpec now carries min_args/max_args instead of argument/bare. One call_form_ok() is shared by evaluation and static validation, so both accept the same call forms. Custom handlers are registered into the process-global FILTER_TABLE over an immutable BUILTIN_FILTERS snapshot. Ordinary handler exceptions become ValueError("<name>: ..."); BaseException passes through.
Loading.run, resume, workflow add and workflow update load functions through one _load_project_workflow_plugins_or_exit helper (which also renders OSError, text and --json). The loader validates every manifest and reports all name collisions (built-in vs. package, package vs. package) before importing any package code. Each package is registered all-or-nothing under a synthetic module name. functions becomes a reserved workflow id.
Install. Rejects names that are built-ins or already provided by another package, whether that package is in the registry or only found on disk (the loader reads the disk, so a registry-only check would let a clashing package in and then break every run).
Shared package mechanics (the largest part of the diff)
Instead of copying the step installer, the kind-independent parts moved into reusable modules, parameterized by small value objects (InstallRequest, PackageSpec, PackageRules, StagingOps; RemoveRequest/RemoveHooks):
workflows/_package_install.py: symlink/path safety, no-follow tree copy with excludes and budgets, project lock, publish-with-rollback, removal, and one staged install_package flow (validate source, stage, re-validate the staged copy, re-run every conflict check under the lock, publish).
workflows/_package_registry.py: PackageRegistry; StepRegistry and FunctionRegistry only name their file/key/error.
workflows/_archive_install.py: the --from URL flow (trust prompt with the URL, GitHub asset resolution, archive-format agreement, safe extract, cleanup).
The step installer keeps every established name as a thin binding, so existing patch points and messages are unchanged and no existing step or bundle test was modified. The module also still loads standalone (.github/scripts/validate_community_workflow_step.py executes it by file path); a test now guards that. See the new "Shared package installation" section in design/workflow-step.md.
The five commits are ordered to review one at a time and each passes the workflow, step and bundle suites on its own: (1) filter arity + registry, (2) shared package mechanics, (3) function loader/installer/wiring, (4) CLI + MCP inventory, (5) docs.
Decisions worth a look
Load-time collisions are an error (plan decision D5), not a silent skip, so a workflow cannot quietly change behavior after an upgrade adds a built-in with the same name.
MCP: the three commands are registered in the inventory as unavailable, like the step commands.
Not in scope: catalog/--version, bundles, variadic arguments, return-type validation.
Follow-ups found while building this (not changed here)
1. Possible bug in existing step loading.
load_custom_steps silently skips a custom step whose type_key is already registered (if type_key in STEP_REGISTRY: continue). A custom step colliding with a built-in or another step is ignored with no error or warning, and when two directories declare the same type_key the winner depends on unsorted iterdir() order. Install-time checks cover new installs only (not hand-copied packages or a built-in added by a later release). This PR deliberately does not copy that behavior for functions; it likely deserves its own issue/PR.
2. Registry records are trusted below the top-level mapping, for steps and functions alike.PackageRegistry only checks that the top-level records value is a mapping. A hand-edited or damaged record is not handled consistently:
specify workflow step list raises AttributeError on a non-mapping record (for example "a": "junk").
specify workflow function list and the install-time name-claim scan raise TypeError when a record's functions value is not a list (for example null).
The CLI never writes such records, and function remove/step remove can clear one. Fixing it (tolerating or reporting damaged records) should be done once for both kinds in the shared registry layer, so it is left for a follow-up instead of diverging functions from steps here.
3. Package imports can reuse stale bytecode, for steps and functions alike._import_package (extracted unchanged from load_custom_steps) loads __init__.py through the default source loader, which trusts a .pyc whose recorded source size and mtime match. An in-place edit with the same byte length inside the same mtime second can therefore keep running the old code until the cache is rewritten; fresh CLI processes read the edited file in every other case. The fix (bypassing or invalidating bytecode for the package and its relative imports, with a same-size/same-second reload test) belongs in the shared helper so steps and functions change together, so it is left for a follow-up.
4. A source directory swapped for a symlink mid-copy is followed, for steps and functions alike.copy_package_tree (moved unchanged from _copy_package_tree) checks each entry with lstat, then later scans the queued directory by pathname. Regular files are opened with O_NOFOLLOW, directories are not, so a writable --dev source that swaps a subdirectory for a symlink between those two operations gets the link's target copied into the package. Unmodified main step code behaves the same. The fix (opening directories with O_NOFOLLOW | O_DIRECTORY and scanning through the descriptor, with a regression test using an injected swap) belongs in the shared helper so steps and functions change together, so it is left for a follow-up.
5. validate_workflow does not statically check filter names or arity in wrapped expressions. Only the bare-condition remediation path consults the filter table, so {{ x | nosuch }} and {{ x | upper(1) }} validate clean and fail at run time. This is the existing behavior for built-in filters on main; functions follow it. Validating wrapped expressions against the filter registry (built-in and custom together, with the loaded plugins) is a separate validator feature and is left for a follow-up.
6. A legacy workflow occupying steps/ or functions/ is scanned for packages, for steps and functions alike.load_custom_steps and load_workflow_functions import every subdirectory of .specify/workflows/<kind>/ that contains the manifest and __init__.py, without checking whether the directory is itself an installed workflow (workflow.yml). steps and functions are reserved workflow ids for new installs, but a workflow installed under that id before the reservation keeps its directory, and a resource subdirectory shaped like a package is imported on run, resume, workflow add and workflow update. For steps this is existing behavior on main; for functions the id was valid before this PR, so the same window exists once. The fix (refusing, with a clear error, when a workflow manifest sits in the scanned directory) belongs in one shared helper so steps and functions change together, so it is left for a follow-up.
Testing
Tested locally with .venv/bin/specify --help
Ran relevant existing tests with .venv/bin/python -m pytest tests -q
Tested with a sample project
All of the following was run by the AI agent (see disclosure):
Full suite: pytest tests -q -n 12 gives 14382 passed, 449 skipped, 1 failed. The failure, tests/integration/test_preset_update_workflow.py::test_preset_update_cli_contract, is pre-existing (it also fails on main without this change and is unrelated).
New tests: ~430 under tests/specify_cli/workflows/function/, mirroring the source layout. They cover the filter arity and handler adapter, manifest parsing, registry parity with StepRegistry (every case runs against both), loader isolation/rollback/collisions, installer (staging, limits, symlinks, concurrent installs, on-disk claims, locked recheck), CLI add/list/remove (including a loopback HTTP fixture for --from), and wiring for run/resume/add/update (text and --json, resume does not replay completed steps).
Manual check in a temp project: function add --dev, function list, workflow run --json using inputs.s | trim | surround('[', ']') (output [hello]), function remove.
AI Disclosure
I did not use AI assistance for this contribution
I did use AI assistance (fill in the disclosure below)
AI disclosure: Implemented with the pi coding agent (via GitHub Copilot) using Claude Sonnet 5.5 at high reasoning effort, running as an agent with the contributor directing the work and confirming each design decision (scope, shared-installer approach, collision behavior) before it was implemented. The agent wrote the code, tests, docs and this description; the contributor did not review every line. Each commit carries an Assisted-by: trailer.
…egistry
Replace FilterSpec.argument/bare with min_args/max_args and derive call-form
validation from one call_form_ok() shared by evaluation and static validation.
Built-in diagnostics keep their hand-written usage text byte for byte.
Add an immutable BUILTIN_FILTERS snapshot and register_custom_filter /
unregister_custom_filter(s) over the process-global FILTER_TABLE. Custom
handlers are called as handler(value, *args) and ordinary exceptions are
normalized to ValueError prefixed with the filter name. The expected-filters
sentence is built from the live table only when an error is raised.
Refs github#5285
Assisted-by: pi coding agent (GitHub Copilot, model: claude-sonnet-5.5, autonomous)
… mechanics
Move the kind-independent parts of step package handling into reusable modules
so another installed package kind does not need a copy:
- _package_install: path/symlink safety, no-follow tree copy with excludes and
budgets, the project lock, publish-with-rollback, removal, and one staged
install_package flow, parameterized by InstallRequest, PackageSpec,
PackageRules and StagingOps value objects.
- _package_registry: PackageRegistry (atomic, symlink-safe JSON registry with
in-memory rollback); StepRegistry now only names its file and key.
- _archive_install: the --from URL flow (trust prompt, download, archive format
agreement, safe extract, cleanup) and a shared error renderer.
- workflows/__init__: extract _import_package/_drop_modules from
load_custom_steps.
The step installer keeps every established name as a thin binding and builds its
spec at call time, so existing patch points, messages and the standalone loading
used by the community step validator are unchanged. Existing step tests are not
modified.
Refs github#5285
Assisted-by: pi coding agent (GitHub Copilot, model: claude-sonnet-5.5, autonomous)
Add .specify/workflows/functions/<id>/ packages (function.yml + __init__.py).
The strict manifest parser, FunctionRegistry (a PackageRegistry subclass),
install rules (export names checked against built-ins, the registry and package
directories found on disk) and the loader are new; staging, copy, locking and
replacement come from the shared package install.
The loader clears the previous project's custom filters, validates every
manifest and reports all collisions before importing any code, imports each
package under a synthetic module name, and registers a package's exports
all-or-nothing. run, resume, workflow add and workflow update load functions
through one _load_project_workflow_plugins_or_exit helper that also renders
OSError. "functions" becomes a reserved workflow id.
Refs github#5285
Assisted-by: pi coding agent (GitHub Copilot, model: claude-sonnet-5.5, autonomous)
specify workflow function add (--dev or --from archive URL, --force), list
(registry metadata only, --json) and remove. add and remove use the shared
trust prompt, download checks, project lock and rollback; list never imports
package code. The three commands are registered in the MCP inventory as
currently unavailable.
Refs github#5285
Assisted-by: pi coding agent (GitHub Copilot, model: claude-sonnet-5.5, autonomous)
Add the workflow functions guide, the custom-filter section in the expressions
reference, the CLI command table and the design notes for the shared package
installation and the function loader.
Refs github#5285
Assisted-by: pi coding agent (GitHub Copilot, model: claude-sonnet-5.5, autonomous)
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Unresolved defects can crash workflow commands or exhaust memory when processing malformed package data, while platform-dependent tests fail on supported CI systems.
Adds project-installed Python callables as custom workflow pipe filters, with management commands and shared package infrastructure reused by custom steps.
Changes:
Adds function manifests, collision-checked loading, and workflow function add/list/remove.
Generalizes filter argument bounds and shares installation, registry, archive, and removal mechanics.
Integrates function loading into workflow commands and adds MCP inventory entries, documentation, and tests.
This pull request requires changes. Unresolved findings cover workflow validation, malformed manifests and registry entries, reserved-name casing, stale bytecode, excessive arity-formatting work, and cross-platform test failures. Additional nits concern diagnostic wording and duplicated package discovery.
A function manifest could declare an arbitrarily large max_args. It passed
install validation and was stored, and building the usage text then allocated
one string per declared argument, so function list and every workflow load
could exhaust memory.
Validate the bounds in one valid_arg_bounds() (0 <= min_args <= max_args <=
MAX_FILTER_ARGS) shared by the manifest parser, register_custom_filter and
function list. Oversized stored values are listed as 0..0 like other damaged
bounds.
Addresses review feedback on github#5303.
Assisted-by: pi coding agent (GitHub Copilot, model: claude-sonnet-5.5, autonomous)
YAML mapping keys can be any scalar. Sorting and joining unknown manifest keys
assumed strings, so a numeric or mixed-type key raised TypeError, which
bypassed the loader's malformed-package handling and the CLI's
WorkflowFunctionError rendering.
Stringify unknown keys before sorting and reporting them, at the manifest and
function-item level.
Addresses review feedback on github#5303.
Assisted-by: pi coding agent (GitHub Copilot, model: claude-sonnet-5.5, autonomous)
- Resolve the expected path in the remove-failure test; macOS and Windows temp
directories are symlinked or 8.3 paths, so the unresolved path never matched
what removal passes to rmtree and the injected error never fired.
- Skip the registry mode-bit assertion on Windows, like the existing
mode-preservation test.
- Run the workflow-wiring shell fixtures through the current interpreter
instead of POSIX printf, single quotes and ';' chaining, which cmd.exe does
not interpret the same way.
Addresses review feedback on github#5303.
Assisted-by: pi coding agent (GitHub Copilot, model: claude-sonnet-5.5, autonomous)
Review round 1: six of the nine inline comments are fixed and three are deferred as shared follow-ups. Head is now 97e3b057.
Fixed
62444c9f: custom filter argument bounds are capped at 16 (0 <= min_args <= max_args <= 16) in the manifest parser, register_custom_filter and function list. A manifest declaring a huge max_args is now rejected at install, and a stored oversized value is listed as 0..0. Regression tests fail on the previous head and pass now.
7ab9570e: non-string keys in function.yml are stringified when reporting unknown keys, so they give the normal unknown key(s) error instead of a TypeError. Covers the manifest and function-item levels, the CLI and the loader.
97e3b057: the four platform-dependent tests are portable. The remove-failure test compares the resolved path (reproduced with a symlinked temp directory, failing before and passing after), the mode-bit test skips on Windows, and the wiring fixtures run Python scripts instead of POSIX printf and ; chaining. Windows and macOS were not run locally; the CI matrix will be the first run there.
Deferred, not changed in this PR
Damaged registry functions collections (command_list.py, installer.py) and stale bytecode in _import_package (loader.py). Steps share both behaviors, so a function-only fix would diverge from them. Both are listed as follow-ups 2 and 3 in the PR description.
Local checks on this head: ruff check src tests passes; the full suite has 14402 passed and 1 failed. The one failure is test_preset_update_cli_contract, which also fails on main.
AI-generated, posted on behalf of @markuswondrak by the pi coding agent (GitHub Copilot, model claude-sonnet-5.5, high reasoning effort, autonomous). The agent wrote the changes, tests and this comment.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
An installer directory-swap race can expose files outside the package source, and wrapped workflow conditions still bypass filter-name and arity validation.
This still expects a validation error after registration because the condition lacks {{ }}; it only tests bare-condition remediation, not the promised workflow-validation contract. A correctly wrapped {{ inputs.s | does_not_exist }} or zero-argument blank() currently passes the condition validator without errors: _evaluator_rejects is reached only from remediation. Add wrapped-workflow cases that assert successful validation for a registered filter and rejection for unknown names/invalid arity, and wire the registry-backed checker into that validation path.
AI-generated review by GitHub Copilot (model: unknown; autonomous, for the review requester).
…d loaded filter
The existing validation test asserted an error after the filter was loaded,
which is the bare-condition wrapping remediation, not a registry rejection;
rename it accordingly and add the wrapped counterpart that must validate clean.
Addresses review feedback on github#5303.
Assisted-by: pi coding agent (GitHub Copilot, model: claude-sonnet-5.5, autonomous)
Review round 2: both new findings are deferred as shared follow-ups; one test-only commit f9ae2ae8 clarifies the validation test. Head is now f9ae2ae8.
Directory swap during copy (installer.py:228): confirmed with an injected swap, and unmodified main step code behaves the same, so it is follow-up 4 in the PR description (fix once in the shared copy helper). Replied on the thread.
Wrapped-condition validation:{{ x | nosuch }} and upper(1) already validate clean on main for built-in filters, because only the bare-condition remediation path consults the filter table. Validating wrapped expressions is a separate validator feature, listed as follow-up 5. The test criticism was fair: the test was renamed to say what it checks, and a wrapped positive case was added (a loaded filter validates clean).
ruff passes and the function tests pass locally; the earlier full-suite result is unchanged apart from this test-only commit.
AI-generated, posted on behalf of @markuswondrak by the pi coding agent (GitHub Copilot, model claude-sonnet-5.5, high reasoning effort, autonomous). The agent wrote the changes, tests and this comment.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Unresolved critical issues permit unintended Python imports from legacy workflow directories and registry-path collisions for mixed-case function IDs on case-insensitive filesystems.
The function-specific reserved-name check is case-sensitive, although validate_step_id checks its reserved names case-insensitively. Function-Registry.Json and FUNCTION-REGISTRY.JSON therefore pass validation. On a case-insensitive filesystem, these IDs alias FunctionRegistry.REGISTRY_FILE: installation publishes a package directory where the JSON registry must be written, then fails during registry persistence. Case-fold the ID before checking this set and add coverage for mixed-case variants.
AI disclosure: GitHub Copilot generated this review and suggestion for the requester (model unavailable; autonomous review).
Avoid importing function packages from legacy workflows
[P1] Avoid treating a legacy workflow as function packages (CWE-829). Before this change, functions was a valid workflow ID, so an installed workflow package can already occupy this directory and contain arbitrary resource subdirectories. After upgrading, any such subdirectory containing function.yml and __init__.py is imported as Python during an unrelated workflow add, update, run, or resume, even if the original workflow was disabled. Reserving the ID for new installations does not protect projects that already have that workflow installed.
This refactor changes a built-in diagnostic, but the PR description says built-in diagnostics stay byte-identical. On main, _check_filter returned "<name>: expected '| <name>' with no arguments or trailing tokens" for a zero-argument filter called with parentheses, such as from_json() or upper(). Now call_form_ok rejects that form, so this function returns the generic "filter 'from_json' used in an unsupported form".
That text reaches users through format_condition_remediation ("the evaluator rejects it (...)"). It also no longer matches _apply_filter, which still raises the zero-argument-specific message at run time. The existing test (test_condition_expression_block.py, "inputs.payload | from_json()") only checks the "the evaluator rejects it" prefix, so it doesn't catch this.
Fix (needed so the claim holds): keep the zero-argument branch in front of the shared check, as _apply_filter already does.
… validation
_check_filter lost the "expected '| name' with no arguments or trailing
tokens" message when it moved to call_form_ok, so `from_json()` and `upper()`
were reported as a generic unsupported form. Restore the message and assert
it in full.
Refs github#5285
Assisted-by: pi coding agent (GitHub Copilot, model: claude-sonnet-5.5, autonomous)
On "Avoid importing function packages from legacy workflows" (loader.py:74): not changing this here.
The exposure is real: a workflow installed as functions before this PR, with a subdirectory holding function.yml and __init__.py, is imported on run, resume, workflow add and workflow update. load_custom_steps has the same property for a pre-existing steps workflow: it never checks for a workflow.yml in the scanned directory, and steps is only reserved for new installs. A guard for functions alone would make the two loaders disagree, so it is tracked as follow-up 6 in the PR description, to be fixed once for both kinds. It needs an existing workflow with that exact id and a package-shaped subdirectory, and that workflow's author already ships code that runs in the project.
The other two findings from that review will be covered in the next round summary.
AI-generated, posted on behalf of @markuswondrak by the pi coding agent (GitHub Copilot, model claude-sonnet-5.5, high reasoning effort), at the contributor's direction. The agent wrote this comment.
On a case-insensitive filesystem `FUNCTION-REGISTRY.JSON` aliases the
registry file, so install published a package directory where the registry
must be written and then failed on save. Compare the id case-insensitively,
as validate_step_id does for step ids.
Refs github#5285
Assisted-by: pi coding agent (GitHub Copilot, model: claude-sonnet-5.5, autonomous)
Review round 3: two findings fixed, one deferred as a shared follow-up. Head is now 138d97d9.
Fixed
70ce3540: a zero-argument filter called with parentheses (from_json(), upper()) in condition validation reports <name>: expected '| <name>' with no arguments or trailing tokens again, as on main and as the run-time path does. The generic "used in an unsupported form" text had replaced it in the arity refactor. The test now asserts the full message. The "built-in diagnostics are byte-identical" sentence is removed from the PR description.
138d97d9: reserved function ids are compared case-insensitively, so Function-Registry.Json and FUNCTION-REGISTRY.JSON are rejected at install instead of aliasing the registry file on case-insensitive filesystems. Regression cases fail without the change and pass with it. Not run on a case-insensitive filesystem.
Deferred, not changed in this PR
A legacy workflow installed under the id functions has its package-shaped subdirectories imported by the loader. load_custom_steps behaves the same for a legacy steps workflow, so a function-only guard would diverge from steps. It is follow-up 6 in the PR description. Replied above.
The function and condition-validation tests pass locally and ruff check is clean on src/specify_cli/workflows and the function tests. The full suite was not re-run on this head.
AI-generated, posted on behalf of @markuswondrak by the pi coding agent (GitHub Copilot, model claude-sonnet-5.5, high reasoning effort), at the contributor's direction. The agent wrote the changes, tests and this comment.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because function discovery can execute previously inert resources from legacy workflows, even when those workflows are disabled.
No open issues 🎉
Review effort: Max (auto)
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds Workflow Functions: project-installed Python callables that workflow expressions can use as pipe filters (
{{ steps.x.output.stdout | trim }}). Closes #5285; the direction and scope were agreed with @mnriem in the issue ("Go for it!").Expression engine.
FilterSpecnow carriesmin_args/max_argsinstead ofargument/bare. Onecall_form_ok()is shared by evaluation and static validation, so both accept the same call forms. Custom handlers are registered into the process-globalFILTER_TABLEover an immutableBUILTIN_FILTERSsnapshot. Ordinary handler exceptions becomeValueError("<name>: ...");BaseExceptionpasses through.Loading.
run,resume,workflow addandworkflow updateload functions through one_load_project_workflow_plugins_or_exithelper (which also rendersOSError, text and--json). The loader validates every manifest and reports all name collisions (built-in vs. package, package vs. package) before importing any package code. Each package is registered all-or-nothing under a synthetic module name.functionsbecomes a reserved workflow id.Install. Rejects names that are built-ins or already provided by another package, whether that package is in the registry or only found on disk (the loader reads the disk, so a registry-only check would let a clashing package in and then break every run).
Shared package mechanics (the largest part of the diff)
Instead of copying the step installer, the kind-independent parts moved into reusable modules, parameterized by small value objects (
InstallRequest,PackageSpec,PackageRules,StagingOps;RemoveRequest/RemoveHooks):workflows/_package_install.py: symlink/path safety, no-follow tree copy with excludes and budgets, project lock, publish-with-rollback, removal, and one stagedinstall_packageflow (validate source, stage, re-validate the staged copy, re-run every conflict check under the lock, publish).workflows/_package_registry.py:PackageRegistry;StepRegistryandFunctionRegistryonly name their file/key/error.workflows/_archive_install.py: the--fromURL flow (trust prompt with the URL, GitHub asset resolution, archive-format agreement, safe extract, cleanup).The step installer keeps every established name as a thin binding, so existing patch points and messages are unchanged and no existing step or bundle test was modified. The module also still loads standalone (
.github/scripts/validate_community_workflow_step.pyexecutes it by file path); a test now guards that. See the new "Shared package installation" section indesign/workflow-step.md.The five commits are ordered to review one at a time and each passes the workflow, step and bundle suites on its own: (1) filter arity + registry, (2) shared package mechanics, (3) function loader/installer/wiring, (4) CLI + MCP inventory, (5) docs.
Decisions worth a look
unavailable, like the step commands.--version, bundles, variadic arguments, return-type validation.Follow-ups found while building this (not changed here)
1. Possible bug in existing step loading.
load_custom_stepssilently skips a custom step whosetype_keyis already registered (if type_key in STEP_REGISTRY: continue). A custom step colliding with a built-in or another step is ignored with no error or warning, and when two directories declare the sametype_keythe winner depends on unsortediterdir()order. Install-time checks cover new installs only (not hand-copied packages or a built-in added by a later release). This PR deliberately does not copy that behavior for functions; it likely deserves its own issue/PR.2. Registry records are trusted below the top-level mapping, for steps and functions alike.
PackageRegistryonly checks that the top-level records value is a mapping. A hand-edited or damaged record is not handled consistently:specify workflow step listraisesAttributeErroron a non-mapping record (for example"a": "junk").specify workflow function listand the install-time name-claim scan raiseTypeErrorwhen a record'sfunctionsvalue is not a list (for examplenull).The CLI never writes such records, and
function remove/step removecan clear one. Fixing it (tolerating or reporting damaged records) should be done once for both kinds in the shared registry layer, so it is left for a follow-up instead of diverging functions from steps here.3. Package imports can reuse stale bytecode, for steps and functions alike.
_import_package(extracted unchanged fromload_custom_steps) loads__init__.pythrough the default source loader, which trusts a.pycwhose recorded source size and mtime match. An in-place edit with the same byte length inside the same mtime second can therefore keep running the old code until the cache is rewritten; fresh CLI processes read the edited file in every other case. The fix (bypassing or invalidating bytecode for the package and its relative imports, with a same-size/same-second reload test) belongs in the shared helper so steps and functions change together, so it is left for a follow-up.4. A source directory swapped for a symlink mid-copy is followed, for steps and functions alike.
copy_package_tree(moved unchanged from_copy_package_tree) checks each entry withlstat, then later scans the queued directory by pathname. Regular files are opened withO_NOFOLLOW, directories are not, so a writable--devsource that swaps a subdirectory for a symlink between those two operations gets the link's target copied into the package. Unmodifiedmainstep code behaves the same. The fix (opening directories withO_NOFOLLOW | O_DIRECTORYand scanning through the descriptor, with a regression test using an injected swap) belongs in the shared helper so steps and functions change together, so it is left for a follow-up.5.
validate_workflowdoes not statically check filter names or arity in wrapped expressions. Only the bare-condition remediation path consults the filter table, so{{ x | nosuch }}and{{ x | upper(1) }}validate clean and fail at run time. This is the existing behavior for built-in filters onmain; functions follow it. Validating wrapped expressions against the filter registry (built-in and custom together, with the loaded plugins) is a separate validator feature and is left for a follow-up.6. A legacy workflow occupying
steps/orfunctions/is scanned for packages, for steps and functions alike.load_custom_stepsandload_workflow_functionsimport every subdirectory of.specify/workflows/<kind>/that contains the manifest and__init__.py, without checking whether the directory is itself an installed workflow (workflow.yml).stepsandfunctionsare reserved workflow ids for new installs, but a workflow installed under that id before the reservation keeps its directory, and a resource subdirectory shaped like a package is imported onrun,resume,workflow addandworkflow update. Forstepsthis is existing behavior onmain; forfunctionsthe id was valid before this PR, so the same window exists once. The fix (refusing, with a clear error, when a workflow manifest sits in the scanned directory) belongs in one shared helper so steps and functions change together, so it is left for a follow-up.Testing
.venv/bin/specify --help.venv/bin/python -m pytest tests -qAll of the following was run by the AI agent (see disclosure):
pytest tests -q -n 12gives 14382 passed, 449 skipped, 1 failed. The failure,tests/integration/test_preset_update_workflow.py::test_preset_update_cli_contract, is pre-existing (it also fails onmainwithout this change and is unrelated).tests/specify_cli/workflows/function/, mirroring the source layout. They cover the filter arity and handler adapter, manifest parsing, registry parity withStepRegistry(every case runs against both), loader isolation/rollback/collisions, installer (staging, limits, symlinks, concurrent installs, on-disk claims, locked recheck), CLI add/list/remove (including a loopback HTTP fixture for--from), and wiring for run/resume/add/update (text and--json, resume does not replay completed steps).ruff check src/specify_cli tests/specify_cli/workflowspasses.function add --dev,function list,workflow run --jsonusinginputs.s | trim | surround('[', ']')(output[hello]),function remove.AI Disclosure
AI disclosure: Implemented with the pi coding agent (via GitHub Copilot) using Claude Sonnet 5.5 at high reasoning effort, running as an agent with the contributor directing the work and confirming each design decision (scope, shared-installer approach, collision behavior) before it was implemented. The agent wrote the code, tests, docs and this description; the contributor did not review every line. Each commit carries an
Assisted-by:trailer.